Skip to content

[WIP] Add "Save as..." command and toolbar button#596

Closed
mfisher87 wants to merge 3 commits intogeojupyter:mainfrom
mfisher87:save-as-in-ui
Closed

[WIP] Add "Save as..." command and toolbar button#596
mfisher87 wants to merge 3 commits intogeojupyter:mainfrom
mfisher87:save-as-in-ui

Conversation

@mfisher87
Copy link
Member

@mfisher87 mfisher87 commented Apr 2, 2025

Description

We want to enable users working on nameless documents (doc = GISDocument()) to save that document from the UI (#594). The current "Save JGIS As..." menu entry from JupyterLab works great in a full view, but not in a widget view or sidecar view.

Checklist

  • PR has a descriptive title and content.
  • PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
  • PR has one of the labels: documentation, bug, enhancement, feature, maintenance
  • Checks are passing.
    Failing lint checks can be resolved with:
    • pre-commit run --all-files
    • jlpm run lint

@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2025

Binder 👈 Launch a Binder on branch mfisher87/jupytergis/save-as-in-ui

@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2025

Integration tests report: appsharing.space

@mfisher87 mfisher87 changed the title [WIP] Init "Save as..." command and toolbar button [WIP] Add "Save as..." command and toolbar button Apr 2, 2025
@mfisher87
Copy link
Member Author

mfisher87 commented Apr 3, 2025

TODO: Add co-authors @tawoodard @YaoTingYao @arjxn-py @martinRenou @rjpolackwich

@mfisher87
Copy link
Member Author

@martinRenou @brichet @gjmooney @simonprovost

I would love to hear your thoughts on this: Right now we can save the file as a new filename and update the model to reflect that. However, changes applied to the widget in the GUI after, for example, adding a new layer, do not get saved to the new filename. "Saving as" a 2nd time, the filename placeholder is properly updated as we expect, but those underlying saves are not happening.

@mfisher87 mfisher87 added enhancement New feature or request help wanted Extra attention is needed labels Apr 7, 2025
mfisher87 and others added 3 commits April 7, 2025 12:46
Co-authored-by: Arjun Verma <arjxn.py@gmail.com>
Co-authored-by: Martin Renou <martin.renou@gmail.com>
Co-authored-by: Yao-Ting Yao <YaoTingYao@users.noreply.github.com>
Co-authored-by: Jamison Polackwich <7376361+rjpolackwich@users.noreply.github.com>
Co-authored-by: Tammy Woodard <19804979+tawoodard@users.noreply.github.com>
Co-authored-by: Arjun Verma <arjxn.py@gmail.com>
Fix the file extension automatically if it doesn't end in jGIS

FIXME: Does not save changes to the shared document after changing the
filePath.

Co-authored-by: Arjun Verma <arjxn.py@gmail.com>
@brichet
Copy link
Collaborator

brichet commented Apr 8, 2025

@mfisher87 I don't think there is a simple way to achieve it.

The file is updated on disk with the shared model of the widget model. I think that, at least, the shared model should be recreated to be able to update the file on disk (probably using this).
Currently the shared model is a read-only property of the widget model. I'm not sure if it would work to change it on the fly (if we make it writable first).

A other solution (maybe easier) could be to replace the whole widget by a new one.

Maybe @davidbrochart or @trungleduc have a better idea about it.

@brichet
Copy link
Collaborator

brichet commented Apr 8, 2025

Thinking again about it, I wonder if it won't be confusing for user to change the shared model on the fly.
When you run the Notebook, you use an unsaved widget. After saving it, you use a widget saved to a file. If you run again the Notebook, you fall back on an unsaved widget, while you could think that your changes will be saved to the disk.

It could be only an export as button for now, to avoid confusion.

@martinRenou
Copy link
Member

martinRenou commented Apr 8, 2025

A other solution (maybe easier) could be to replace the whole widget by a new one.

It looks to me that this is what JupyterLab does already with notebooks when you run save-as, since I see the widget blink (as if we closed the widget and reopened it). To be verified in the code itself.

@brichet
Copy link
Collaborator

brichet commented Apr 8, 2025

Another way to avoid confusion could be to have 2 classes:

  • GISDocument requiring a filename at instantiation and backed by a file
  • GISSandbox ( or GISExplore, to reuse the suggestion of @fperez about the "bounce" function), that is never backed by a file. It could still export the project to a file, without synchronisation after export.

@mfisher87
Copy link
Member Author

@brichet Interesting idea! Fernando compared this dynamic to similar core concepts in JupyterLab: a Notebook, which is file-backed at all times, and a console, which is ephemeral, not file-backed. When a user opens a console vs a Notebook, they look significantly different and visual cues enable the user to reason about the difference between the two environments.

Do you have thoughts on what visual cues might help the user differentiate between those different classes and how they must use them differently? For example, if calling an explore function from a Notebook opens an interface using GISSandbox that looks similar to an interface using GISDocument, how does the user know that they must behave differently to save their work?

@brichet
Copy link
Collaborator

brichet commented Apr 11, 2025

Do you have thoughts on what visual cues might help the user differentiate between those different classes and how they must use them differently?

I don't know, but I agree that it has to be obvious that it's not saved.

Maybe an additional item in the toolbar and/or a border to the widget.

@mfisher87
Copy link
Member Author

There's a "Save as..." method in the "File" menu. Let's punt this until later :)

@mfisher87 mfisher87 closed this May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants